Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes #138164

jdonszelmann · 2025-03-07T14:43:22Z

This PR adds a new field to OwnerInfo to buffer lints which are generated during attribute parsing and ast lowering in general. They can't be emitted at this stage because at that point there's no HIR yet, and early lints are already emitted.

This also adds the generic S: Stage to attribute parsers. Currently we don't emit any lints during early attribute parsing, but if we ever want to that logic will be different. That's because there we don't have hir ids yet, while at the same time still having access to node ids and early lints. Even though that logic isn't completely there in this PR (no worries, we don't use it), that's why the parameter is there.

With this PR, we also add 2 associated consts to SingleAttributeParser. Those determine what logic should be applied when finding a duplicate attribute.

This PR was getting pretty large, so the first code using this logic is in #138165. This code is all new things that weren't possible before so it also doesn't break any behaviour. However, some of it will be dead code right now. I recommend reviewing both before merging, though in some sense that doubles the size of the review again, and the other PR might be more controversial. Let me know how you want to do this @oli-obk

rustbot · 2025-03-07T14:43:30Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

jdonszelmann · 2025-03-07T14:45:51Z

(am aware of merge commits, was just an easy way to get those changes in before they're on master so we can run CI on this PR. will obviously remove once those changes are on master and this PR can be merged)

compiler/rustc_errors/src/lib.rs

bors · 2025-03-12T12:10:09Z

☔ The latest upstream changes (presumably #138388) made this pull request unmergeable. Please resolve the merge conflicts.

jdonszelmann · 2025-06-09T22:54:31Z

@rustbot review

jdonszelmann · 2025-06-12T10:10:02Z

@bors r=oli_obk

bors · 2025-06-12T10:10:05Z

📌 Commit e2afe04 has been approved by oli_obk

It is now in the queue for this repository.

fmease · 2025-06-12T10:18:21Z

@bors r=oli-obk

bors · 2025-06-12T10:18:23Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Rollup of 9 pull requests #142399

bors · 2025-06-12T10:18:24Z

📌 Commit e2afe04 has been approved by oli-obk

It is now in the queue for this repository.

…ra, r=oli-obk Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes r? `@oli-obk` This PR adds a new field to OwnerInfo to buffer lints which are generated during attribute parsing and ast lowering in general. They can't be emitted at this stage because at that point there's no HIR yet, and early lints are already emitted. This also adds the generic `S: Stage` to attribute parsers. Currently we don't emit any lints during early attribute parsing, but if we ever want to that logic will be different. That's because there we don't have hir ids yet, while at the same time still having access to node ids and early lints. Even though that logic isn't completely there in this PR (no worries, we don't use it), that's why the parameter is there. With this PR, we also add 2 associated consts to `SingleAttributeParser`. Those determine what logic should be applied when finding a duplicate attribute. This PR was getting pretty large, so the first code using this logic is in rust-lang#138165. This code is all new things that weren't possible before so it also doesn't break any behaviour. However, some of it will be dead code right now. I recommend reviewing both before merging, though in some sense that doubles the size of the review again, and the other PR might be more controversial. Let me know how you want to do this `@oli-obk`

Rollup of 10 pull requests Successful merges: - #134536 (Lint on fn pointers comparisons in external macros) - #138164 (Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes) - #141069 (Suggest mut when possbile for temporary value dropped while borrowed) - #141934 (resolve: Tweak `private_macro_use` lint to be compatible with upcoming macro prelude changes) - #142034 (Detect method not being present that is present in other tuple types) - #142402 (chore(doctest): Remove redundant blank lines) - #142406 (Note when enum variants shadow an associated function) - #142407 (Remove bootstrap adhoc group) - #142408 (Add myself (WaffleLapkin) to review rotation) - #142418 (Remove lower_arg_ty as all callers were passing `None`) r? `@ghost` `@rustbot` modify labels: rollup

workingjubilee · 2025-06-12T18:17:55Z

There are several open PRs touching adjacent areas that will conflict with this, and there is somewhat-timing-sensitive work that is strictly blocked on this. Bumping its priority accordingly.

@bors p=1 rollup=iffy

workingjubilee · 2025-06-12T18:18:45Z

( Ideally it just lands in the rollup currently trying to land though. )

bors · 2025-06-12T19:48:52Z

⌛ Testing commit e2afe04 with merge 573a015...

jdonszelmann · 2025-06-12T20:31:11Z

@workingjubilee I'm happy this gets merged but some of those I deliberately didn't r+ yet to wait for this one but this makes sure it work :)

bors · 2025-06-12T23:04:41Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 573a015 to master...

github-actions · 2025-06-12T23:07:30Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 6c8138d (parent) -> 573a015 (this PR)

Test differences

Show 147 test diffs

Stage 1

session_diagnostics::verify_attr_parsing_empty_confusables_30: pass -> [missing] (J0)
session_diagnostics::verify_attr_parsing_empty_confusables_31: [missing] -> pass (J0)
session_diagnostics::verify_attr_parsing_repr_ident_31: pass -> [missing] (J0)
session_diagnostics::verify_attr_parsing_repr_ident_32: [missing] -> pass (J0)
session_diagnostics::verify_attr_parsing_stability_outside_std_29: pass -> [missing] (J0)
session_diagnostics::verify_attr_parsing_stability_outside_std_30: [missing] -> pass (J0)
session_diagnostics::verify_attr_parsing_unrecognized_repr_hint_32: pass -> [missing] (J0)
session_diagnostics::verify_attr_parsing_unrecognized_repr_hint_33: [missing] -> pass (J0)
session_diagnostics::verify_attr_parsing_unused_duplicate_29: [missing] -> pass (J0)

Additionally, 138 doctest diffs were found. These are ignored, as they are noisy.

Job group index

J0: aarch64-apple, aarch64-gnu, i686-gnu-2, i686-gnu-nopt-2, i686-msvc-2, x86_64-apple-1, x86_64-gnu, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-3, x86_64-gnu-llvm-20-1, x86_64-gnu-llvm-20-3, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-2, x86_64-msvc-2

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 573a01569000d395498a5f98f916d6e5305ac81a --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-2: 5058.2s -> 6576.6s (30.0%)
x86_64-apple-1: 7328.3s -> 9029.2s (23.2%)
dist-x86_64-msvc-alt: 7179.2s -> 8838.4s (23.1%)
dist-various-1: 4293.5s -> 4917.7s (14.5%)
dist-x86_64-apple: 9693.9s -> 10934.9s (12.8%)
dist-android: 2388.0s -> 2613.8s (9.5%)
dist-apple-various: 6971.2s -> 6390.0s (-8.3%)
dist-x86_64-solaris: 5465.3s -> 5020.7s (-8.1%)
dist-aarch64-linux: 6155.9s -> 5682.4s (-7.7%)
x86_64-mingw-1: 9148.9s -> 8527.6s (-6.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-06-13T00:31:31Z

Finished benchmarking commit (573a015): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.1%, 1.0%]	107
Regressions ❌ (secondary)	1.0%	[0.1%, 3.1%]	56
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.5%	[0.1%, 1.0%]	107

Max RSS (memory usage)

Results (primary 1.7%, secondary 3.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.4%, 7.3%]	9
Regressions ❌ (secondary)	4.4%	[1.0%, 13.6%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-8.5%	[-8.5%, -8.5%]	1
All ❌✅ (primary)	1.7%	[0.4%, 7.3%]	9

Cycles

Results (primary -1.0%, secondary 1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[1.6%, 2.7%]	6
Improvements ✅ (primary)	-1.0%	[-1.0%, -1.0%]	1
Improvements ✅ (secondary)	-1.6%	[-1.6%, -1.6%]	1
All ❌✅ (primary)	-1.0%	[-1.0%, -1.0%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 754.987s -> 754.801s (-0.02%)
Artifact size: 372.08 MiB -> 372.21 MiB (0.03%)

…-perf-problems, r=<try> collect delayed lints in hir_crate_items r? `@oli-obk` Attempt to mitigate perf problems in #138164

rustbot assigned oli-obk Mar 7, 2025

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 7, 2025

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 7, 2025

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the attr-parsing-lint-infra branch from b5f1130 to 9559a37 Compare March 8, 2025 19:03

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 8, 2025

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the attr-parsing-lint-infra branch from 9559a37 to d529827 Compare March 9, 2025 15:31

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the attr-parsing-lint-infra branch from d529827 to bfeed70 Compare March 9, 2025 16:08

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the attr-parsing-lint-infra branch from bfeed70 to fc63551 Compare March 9, 2025 20:08

oli-obk reviewed Mar 10, 2025

View reviewed changes

compiler/rustc_errors/src/lib.rs Outdated Show resolved Hide resolved

oli-obk reviewed Mar 10, 2025

View reviewed changes

compiler/rustc_errors/src/lib.rs Outdated Show resolved Hide resolved

oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2025

wesleywiser mentioned this pull request Apr 24, 2025

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Open

jdonszelmann force-pushed the attr-parsing-lint-infra branch 2 times, most recently from 2be9ade to 57ee80c Compare June 9, 2025 22:40

jdonszelmann mentioned this pull request Jun 9, 2025

Rewrite inline attribute parser to use new infrastructure and improve diagnostics for all parsed attributes #138165

Open

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 9, 2025

jdonszelmann added 2 commits June 12, 2025 09:56

add error message for unused duplicate

e6a5f28

remove 'static in some places

e2afe04

jdonszelmann force-pushed the attr-parsing-lint-infra branch from 92205a2 to e2afe04 Compare June 12, 2025 08:02

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 12, 2025

This was referenced Jun 12, 2025

Lexer: check in advance_token to avoid regard spare ## as GardedStrPrefix #141028

Open

Refactor rustc_attr_data_structures documentation #142082

Open

matthiaskrgr mentioned this pull request Jun 12, 2025

Rollup of 10 pull requests #142422

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 12, 2025

bors merged commit 573a015 into rust-lang:master Jun 12, 2025
11 checks passed

rustbot added this to the 1.89.0 milestone Jun 12, 2025

bors mentioned this pull request Jun 12, 2025

[perf] Decouple directly accessing a HIR owner from ast lowering #138705

Draft

rustbot added the perf-regression Performance regression. label Jun 13, 2025

jdonszelmann mentioned this pull request Jun 13, 2025

collect delayed lints in hir_crate_items #142455

Open

bors added a commit that referenced this pull request Jun 13, 2025

Auto merge of #142455 - jdonszelmann:attempt-to-mitigate-delayed-lint…

437cd00

…-perf-problems, r=<try> collect delayed lints in hir_crate_items r? `@oli-obk` Attempt to mitigate perf problems in #138164

fmease mentioned this pull request Jun 13, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes #138164

Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes #138164

Conversation

jdonszelmann commented Mar 7, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 7, 2025

Uh oh!

This comment has been minimized.

jdonszelmann commented Mar 7, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

bors commented Mar 12, 2025

Uh oh!

jdonszelmann commented Jun 9, 2025

Uh oh!

jdonszelmann commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

fmease commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

workingjubilee commented Jun 12, 2025

Uh oh!

workingjubilee commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

jdonszelmann commented Jun 12, 2025 • edited by jieyouxu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Jun 12, 2025

Uh oh!

Uh oh!

github-actions bot commented Jun 12, 2025

Test differences

Stage 1

Job duration changes

Uh oh!

rust-timer commented Jun 13, 2025

Overall result: ❌ regressions - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

jdonszelmann commented Mar 7, 2025 •

edited by rustbot

Loading

jdonszelmann commented Jun 12, 2025 •

edited by jieyouxu

Loading